Skip to content

Conversation

dloukadakis
Copy link
Contributor

@dloukadakis dloukadakis commented Oct 5, 2025

Objective

Fixes #8906

#8906 reports that the reflect attribute silently fails when using the #[reflect[...]] syntax. I wasn't able to reproduce that issue, so it was likely resolved in a prior change. This pull request addresses only the syntax issue.

Solution

Validate the Meta::List delimiter in reflect macro to allow only MacroDelimiter::Paren

Testing

  • Did you test these changes? If so, how?
    I run cargo check --example reflection_types
  • Are there any parts that need more testing?
    No
  • How can other people (reviewers) test your changes? Is there anything specific they need to know?
    Run cargo check --example reflection_types

Then modify the reflection_types example by changing:

#[reflect(Hash, PartialEq, Clone)]
pub struct E {
    x: usize,
}

to

#[reflect[Hash, PartialEq, Clone]]
pub struct E {
    x: usize,
}

then run cargo check --example reflection_types again it should now fail with this error:

error: `#[reflect = "..."]` must use parenthesis `(` and `)`

@dloukadakis dloukadakis added C-Bug An unexpected or incorrect behavior A-Reflection Runtime information about types D-Macros Code that generates Rust code M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 5, 2025
Copy link
Contributor

github-actions bot commented Oct 5, 2025

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Please review the instructions for writing migration guides, then expand or revise the content in the migration guides directory to reflect your changes.

@dloukadakis dloukadakis force-pushed the bugfix/reflect-parenthesis branch from ff47f84 to e14c76d Compare October 5, 2025 17:24
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good fix, but I don't think that the migration guide is correct. It should instead focus on communicating that "this didn't work before either, but it led to silent bugs".

@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged D-Straightforward Simple bug fixes and API improvements, docs, test and examples and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 5, 2025
@dloukadakis
Copy link
Contributor Author

dloukadakis commented Oct 5, 2025

#8906 reports that the reflect attribute silently fails when using the #[reflect[...]] syntax. I wasn't able to reproduce that issue, so it was likely resolved in a prior change. This pull request addresses only the syntax issue.

@alice-i-cecile I initially forgot to clarify why this pull request focuses only on the syntax, please see the updated pull request description.

@dloukadakis dloukadakis added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Oct 6, 2025
@dloukadakis dloukadakis force-pushed the bugfix/reflect-parenthesis branch from e14c76d to 8cc80d7 Compare October 6, 2025 01:17
@dloukadakis dloukadakis changed the title Allow only parenthesis in #[reflect(...)] Allow only parentheses in #[reflect(...)] Oct 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Reflection Runtime information about types C-Bug An unexpected or incorrect behavior D-Macros Code that generates Rust code D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn or throw error to user about typo in reflect container attributes
2 participants